-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Basin / subgrid_time
table
#1975
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you document the construction of the new interpolation objects? That inner loop is opaque to me (with complexted indexlookups etc).
core/src/read.jl
Outdated
cache_parameters = true, | ||
) | ||
# # These should only be pushed when the subgrid_id has changed | ||
if subgrid_id_time[end] != subgrid_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, not sure if I get it, but if we group by subgrid_id, this will always be true?
Friday afternoon eh. You better not make a release after this, but enjoy the weekend 🎉 |
Fixes #1010
This adds
Basin / subgrid_time
. So far the only relation we could update over time wasQ(h)
(TabulatedRatingCurve / time
), and that is implemented differently. I wrote #1976 to get those more in line. It's good to read that since it explains the implementation here.Things I dislike:
Basin / subgrid_time
, just likeBasin / concentration_
.Basin / static
,Basin / time
. Since we already haveBasin / subgrid
we cannot do that.